Skip to content

SearchFragment: show service name in search hint #12258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Profpatsch
Copy link
Contributor

@Profpatsch Profpatsch commented May 7, 2025

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

The only hint (haha) which service one is searching in is currently the color of the background. This is super confusing, yesterday a friend tried to search for a video on youtube and the app was set to Bandcamp, and they were super confused why nothing turned up.

So let’s put the name of the service in the hint!

The updateService() thing is a little confused, but I didn’t want to refactor to improve the logic. It’s not doing anything computationally intensive anyway.

For PeerTube, the sidebar calls it FramaTube but the service name is PeerTube, I’m not sure why that is the case. Looks like the string depends on the name of the instance? Hm, can be improved later I think.

Before/After Screenshots/Screen Record

after:

newpipe_search_service_name.mp4

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR. You can find more info and a video demonstration on this wiki page.

Due diligence

The only hint (haha) which service one is searching in is currently
the color of the background. This is super confusing, yesterday a
friend tried to search for a video on youtube and the app was set to
Bandcamp, and they were super confused why nothing turned up.

So let’s put the name of the service in the hint!

The `updateService()` thing is a little confused, but I didn’t want
to refactor to improve the logic. It’s not doing anything
computationally intensive anyway.

For PeerTube, the sidebar calls it FramaTube but the service name is
PeerTube, I’m not sure why that is the case. Looks like the string
depends on the name of the instance? Hm, can be improved later I
think.
@github-actions github-actions bot added the size/small PRs with less than 50 changed lines label May 7, 2025
@Profpatsch Profpatsch requested a review from Copilot May 7, 2025 08:19
Copilot

This comment was marked as off-topic.

Copy link

sonarqubecloud bot commented May 7, 2025

@AudricV AudricV added feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface multiservice Issues related to multiple services search Anything related to the search function labels May 7, 2025
Comment on lines +227 to +229
searchEditText.setHint(
getString(R.string.search_with_service_name,
service.getServiceInfo().getName()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it will be more useful to also include the selected content filter (e.g. Youtube Music) in the hint. WDYT

@ShareASmile ShareASmile added the waiting for author If the author doesn't respond, the issue will be auto-closed. Otherwise the label will be removed. label Jun 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface multiservice Issues related to multiple services search Anything related to the search function size/small PRs with less than 50 changed lines waiting for author If the author doesn't respond, the issue will be auto-closed. Otherwise the label will be removed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants